[AUDIT SINK PART 2] File implements audit Sink Trait - #1190
Merged
Merged
Conversation
HatemMn
marked this pull request as ready for review
September 11, 2026 15:44
HatemMn
added this pull request to stack #1128
September 11, 2026 15:47
HatemMn
removed this pull request from stack #1128
September 11, 2026 15:49
HatemMn
added this pull request to stack #1196
September 15, 2026 08:52
HatemMn
force-pushed
the
feat/audit-sink-generalize
branch
from
September 15, 2026 12:53
d3c0bee to
33a2aec
Compare
HatemMn
force-pushed
the
feat/audit-sink-generalize
branch
from
September 15, 2026 13:49
33a2aec to
94866c5
Compare
Manuthor
removed this pull request from stack #1196
September 15, 2026 16:28
HatemMn
added this pull request to stack #1200
September 16, 2026 12:33
HatemMn
force-pushed
the
feat/audit-sink-generalize
branch
from
September 16, 2026 14:18
8ac88da to
b51becd
Compare
HatemMn
force-pushed
the
feat/audit-sink-refactor
branch
from
September 16, 2026 14:18
e9c0a4b to
c92a46c
Compare
HatemMn
had a problem deploying
to
xks-remote-approval
September 16, 2026 14:19 — with
GitHub Actions
Error
Manuthor
reviewed
Sep 16, 2026
Manuthor
left a comment
Contributor
There was a problem hiding this comment.
Code review for PR #1190 (feat/audit-sink-generalize against feat/audit-sink-refactor):
The generalization of AuditStore over the async AuditSink trait is clean and sound. Moving recovery and exclusive lock handling into FileSink::resume() preserves the non-blocking startup contract. Below are specific in-scope suggestions.
Replace the private, file-only mock AuditSink trait with the public async cosmian_kms_interfaces::AuditSink defined in the previous commit: - FileSink: AuditSink wraps the file backend (lock+recover retry loop moves from writer_supervisor into FileSink::resume(), never returns Err in practice — preserves the always-start/self-heal behavior exactly). - write_failure_is_fatal() = false (preserves the file backend's historical log-and-continue behavior). - Added AuditSink::is_write_capacity_exceeded() (default false) so the generic writer loop can skip a write without an error-per-event log spam; FileSink is the only implementor that ever returns true (max_size_bytes cap). - writer_loop/write_draft_to_chain are now generic over any AuditSink and fully backend-agnostic (no file-specific cap/path knowledge left). - Added AuditEventDraft::finalize() (cosmian_kms_access) to share the draft->AuditEvent+row_hash computation between the async steady-state writer and the sync recovery-time sentinel writer (torn-write/reanchor), which runs inside spawn_blocking with no async runtime available. - AuditFileStore's public API (start/start_with_max_size/enqueue/flush/ new_disconnected) is unchanged; no consumer (middleware, kms/mod.rs, tests) needed changes. All 47 audit tests (access + core::audit + middleware + integration) green, clippy clean, no behavior change observed in file-backend tests.
update_log_index.py --check was failing on this branch: 2 stale entries (superseded by the resume() rewrite) and 4 new entries (from the earlier resume()/store.rs fixes) were undocumented. Not caused by the rebase — pre-existing gap from an earlier direct fix that never ran the doc-sync tool.
…unparsable Caught by the typos pre-commit hook (not installed until now, hence never enforced on this stack). Same misspelling already exists on develop's file_store.rs, unrelated to this rebase. The one other repo occurrence (crate/clients/clap/src/actions/audit.rs) is a static test fixture string, not coupled to this enum — left untouched.
HatemMn
force-pushed
the
feat/audit-sink-refactor
branch
from
September 21, 2026 13:08
c92a46c to
7f381ec
Compare
HatemMn
force-pushed
the
feat/audit-sink-generalize
branch
from
September 21, 2026 13:08
b51becd to
90dce08
Compare
Manuthor
reviewed
Sep 22, 2026
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
feat/audit-sink-generalizeOne commit. This is where the file backend actually adapts to the new public
AuditSinktrait instead of a private mock one.Small diff but this is the one place old behavior could have silently shifted during the refactor.
Got its own PR instead of getting buried in the split commit before it.